Repository navigation
feat: En 206 move provider platforms - #1284
Merged
Merged
Conversation
|
Important Review skippedToo many files! This PR contains 135 files, which is 35 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.
 |
github-actions
Bot
added
documentation
CK-7vn
changed the title
carddev81
requested changes
Nov 4, 2026
carddev81
left a comment
carddev81
left a comment
Contributor
There was a problem hiding this comment.
@CK-7vn, awesome job! Here is some findings:
- Merging this breaks staging.
Since EN-184, every main build gets pinned on staging. So as soon as this merges, staging runs the new core withcomponents.providerPlatformsstill off:
providerplatforms-workerrunsbackend worker providerplatformson the new image. That module is gone, sofindWorkerhitslog.Fataland the pod crash-loops.- Every Provider Platforms route answers 502, because
PROVIDER_PLATFORMS_URLis empty.
The site-file switch in step 5 has to land together with this merge, not afterwards. Please write the exact staging steps in the description. On EN-205, the platform PR turned staging on itself (see my platform #20 review).
- Business logic in the database layer.
DrainMappingDeletionsloops over the queue, calls the service, and decides what stays queued. The database package should only read and delete rows; the loop belongs intasks:
// database
func (db *DB) QueuedMappingDeletions(ctx context.Context) ([]uint, error)
func (db *DB) DequeueMappingDeletion(ctx context.Context, userID uint) error
// tasks.unlinkDeletedUsers: list, call DeleteUserMappings, dequeue on success, log the rest- Token and URL don't match how core handles Open Content.
For Open Content, core readsOPEN_CONTENT_TOKEN, defaults the URL tohttp://open-content:8080, and fails to start without the token. Here it's a sharedSERVICE_TOKENwith no default URL and no startup check. A missing URL turns every Provider Platforms route into a 502, and a missing token turns every call into a 401, and neither gets logged at startup. Please match Open Content:
PROVIDER_PLATFORMS_TOKENPROVIDER_PLATFORMS_URLdefaulting tohttp://provider-platforms:8080log.Fatalwhen the token is empty, outside tests
- Compose clashes with Open Content.,
- Ports:
open-content(#1281) andproviderplatformsboth bind host port 8082, and.env.examplepoints both URLs at127.0.0.1:8082. With both merged,make devwon't start. Use 8083. - Build source: follow Open Content's compose setup. It builds from GitHub by default, with a
*_DIR/*_REFoverride (thex-open-contentanchor and the Makefile lines). Here,make devfails unless you have../unlocked-provider-platformschecked out.
- Make targets skip the service.
make migrate,make migrate-freshandmake seeddon't run the service's migrate or seed. For Open Content they run itsmigrate,migrate-resetandseed-dev; please do the same here (this needs theseed-devfrom my PP #1 review).- The
make canvasbanner still says "edit the seeded Canvas provider platform", and nothing seeds one anymore.
- Rebase, and order this against EN-205.
- This branch is on
4398d866; main is at9dd57e8c. - #1281 (EN-205) touches the same files:
registry.go,server.go,docker-compose.yml,.env.exampleandgo.mod. Say in the description which one merges first. - Migration
00079also clashes with my #1282, which adds a00079too. Whichever lands second renumbers; if yours goes first, I'll renumber mine.
- The ticket can only close once all of this is done:
- Contracts #7 is merged and tagged
v0.4.0, with the pin swapped here and in unlocked-provider-platforms. - unlocked-provider-platforms #1 is merged, with its image in ECR. Main needs branch protection; I'll set that.
- unlocked-platform #20 is merged.
- Staging has been switched over, and LMS sign-in and a sync have both been checked there.
… the scheduler (EN-206) Deleting a user no longer writes into Provider Platforms' tables in core's transaction, since Provider Platforms is becoming its own service. It queues a row in the same transaction, and a job unlinks the user once Provider Platforms has done it.
Core stops running the Provider Platforms module and talks to the service instead. - The module's 30 /api routes are forwarded to the service after the same session, role, feature and user checks, with the service token and the caller's claims (providerplatforms.Routes in unlocked-contracts). - registry.ProviderPlatforms is now the contracts HTTP client, so Programs, the dashboard and the user routes ask the service. Learning Record is unchanged. - Core serves /internal/v1/core/ (coreapi) for the service's calls to core's users and Programs' live providers. - The module, its api package, its core integration tests and its depguard rule are gone; the seeder no longer seeds provider platforms or learning data, which the service's demo seed does. - The Programs integration tests run against a fake service. - Needs unlocked-contracts v0.4.0.
…(EN-206) Compose builds the service from the checkout next to this one, as providerplatforms, providerplatforms-worker and a one-shot migrate that runs after core's. The server and scheduler get PROVIDER_PLATFORMS_URL and the shared SERVICE_TOKEN.
…EN-206) testenv.go is not a _test file, so go build ./... (CI's every-module build) failed on startFakeProviderPlatforms, which only existed in a _test file.
…(EN-206) v0.4.0 is not tagged yet, so no module could resolve it. Pins backend, seeder and migrations to unlocked-contracts#7's head commit and tidies away the dependencies only the removed module used. Switch to v0.4.0 once it is tagged.
…he service (EN-206) Sends the contracts' routes through core's router to a stand-in service: a user reading another user's data, a facility admin reading another facility's user, a student on an admin route and a facility without the feature are refused, and none of those requests reaches the service.
…ow sets up Kolibri (EN-206)
…t a factory (EN-206)
CK-7vn
force-pushed
the
EN-206-move-provider-platforms
branch
from
November 4, 2026 18:56
95f252e to
087fd0e
Compare
carddev81
self-requested a review
November 4, 2026 19:49
carddev81
requested changes
Nov 4, 2026
carddev81
left a comment
carddev81
left a comment
Contributor
There was a problem hiding this comment.
@CK-7vn thanks for make the changes, there seems to be some of the same issues...so I'm not sure this was ready or not. I seen that you made changes though
-
DrainMappingDeletionsis still business logic in the database layer.
It's still indatabase/mapping_deletions.go, looping and calling the service. Split it as I described:QueuedMappingDeletionsandDequeueMappingDeletionindatabase, with the loop intasks. The "No business logic exists within the database layer" box in the description is ticked, so please untick it until this is moved. -
Token and URL still don't match Open Content.
It's stillSERVICE_TOKEN, with no default URL and no startup check (registry.go:42-45). Open Content's version is on main now, so you can copy it:PROVIDER_PLATFORMS_TOKEN,PROVIDER_PLATFORMS_URLdefaulting tohttp://provider-platforms:8080, andlog.Fatalwhen the token is empty, outside tests. -
Compose still clashes with Open Content, and now it's on main.
providerplatformsstill binds host port 8082, whichopen-contentuses on main. Use 8083, here and in.env.example.- It still builds only from
../unlocked-provider-platforms. Follow main'sx-open-contentanchor and its Makefile lines: build from GitHub by default, with aPROVIDER_PLATFORMS_DIR/PROVIDER_PLATFORMS_REFoverride.
-
Make targets still skip the service.
make migrate,make migrate-freshandmake seedstill only run core's. Main's Makefile now runs Open Content's migrate, migrate-reset and seed-dev, so add the same for Provider Platforms. Themake canvasbanner still says "edit the seeded Canvas provider platform". -
Rebase onto main again.
EN-205 (#1281) merged first, and 1.9.0 is out, so main is at 55c9ff0. This branch is on 9dd57e8 and conflicts in 19 files, includingregistry.go,server.go,handlers/modules.go,module/module.go,docker-compose.yml,.env.exampleand the threego.mods. Open Content isn't a module in core anymore, soregistry.All()and the tests built around it need to follow main.
Migration 00079 still clashes with my #1282. Whichever lands second renumbers.
…platforms # Conflicts: # .env.example # backend/.golangci.yml # backend/go.mod # backend/go.sum # backend/migrations/go.mod # backend/migrations/go.sum # backend/seeder/go.mod # backend/seeder/go.sum # backend/src/database/seed_demo.go # backend/src/handlers/internal_features.go # backend/src/handlers/modules.go # backend/src/handlers/modules_test.go # backend/src/handlers/server.go # backend/src/models/dashboard.go # backend/src/modules/module/module.go # backend/src/modules/opencontent/README.md # backend/src/modules/registry/registry.go # backend/src/modules/registry/registry_test.go # backend/tests/integration/testenv.go # docker-compose.yml
…PEN_CONTENT_TOKEN (EN-206)
…ts table now has (EN-206)
carddev81
requested changes
Nov 5, 2026
carddev81
left a comment
carddev81
left a comment
Contributor
There was a problem hiding this comment.
@CK-7vn nice, this round got everything from last time. One thing before this merges:
make devbreaks for everyone once this merges. Compose builds Provider Platforms fromhttps://github.com/UnlockedLabs/unlocked-provider-platforms.gitby default, but that repo is private. Docker can't clone it without credentials, somake devfails unlessPROVIDER_PLATFORMS_DIRis set. Open Content and contracts are both public, which is why Open Content's build works. The repo needs to be public before this merges (I'll take care of that), and then the "The repo is private for now..." line comes out of the description.
After contracts #7 is tagged, swap the pin from v0.3.2-0.20261105174553-f3516cf56ee0 to v0.4.0.
EN-206 can only close once:
- contracts #7 is merged and tagged
v0.4.0, with the pin swapped here and in unlocked-provider-platforms#1; - unlocked-provider-platforms#1 is merged, with its image in ECR, and the repo is public;
PROVIDER_PLATFORMS_TOKENis in staging'score-secrets;- unlocked-platform#20 is merged with staging turned on, and this PR is merged right after it deploys;
- on staging: LMS sign-in and a sync both work, the syncs run once,
providerplatforms-workeris gone, and the Learning Record output is unchanged.
…oes public before this merges (EN-206)
carddev81
self-requested a review
November 5, 2026 19:40
carddev81
approved these changes
Nov 5, 2026
carddev81
left a comment
carddev81
left a comment
Contributor
There was a problem hiding this comment.
@CK-7vn great job!!
Squash this when you merge. The main merge brought a 40MB backend/migrations/migrations binary into the branch history. f42d49b0 removes it, but a rebase merge would still carry it into main.
Hold the merge until unlocked-platform#20 has deployed to staging, per the merge order in the description.
EN-206 can only close once:
- unlocked-provider-platforms#1 is merged, with its image in ECR;
PROVIDER_PLATFORMS_TOKENis in staging'score-secrets;- unlocked-platform#20 is merged with staging turned on, and this PR is merged right after it deploys;
- on staging: LMS sign-in and a sync both work, the syncs run once,
providerplatforms-workeris gone, and the Learning Record output is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pre-Submission PR Checklist
Description of the change
Core stops running the Provider Platforms module and talks to the new unlocked-provider-platforms service instead.
Core stays the public edge. The module's 30
/apiroutes go through the same session, role, feature and user checks they had in core, from the contracts'providerplatforms.Routestable. Core then forwards them to the service withAuthorization: Bearer <PROVIDER_PLATFORMS_TOKEN>and the caller's claims header. It removes the browser's cookie and any claims header the browser sent. Ingress doesn't change.registry.ProviderPlatformsis now the contracts HTTP client, so Programs, the dashboard and the user routes ask the service. Learning Record is unchanged.Core serves
/internal/v1/core/(coreapi) for the service's calls back to core: users (get, create a student, create login, delete) and Programs' live providers. It's guarded by the same token.User delete no longer writes Provider Platforms' tables in core's transaction.
DeleteUseradds a row to the newpublic.provider_mapping_deletionstable (migration00080) in that same transaction. A scheduler job intasksruns every minute: it lists the queue (database.QueuedMappingDeletions), asks the service to unlink each user, and dequeues the ones it unlinked (database.DequeueMappingDeletion). A user the service can't unlink yet stays queued until the next run.Removed from core: the module, its
apipackage, its integration tests and goldens (they moved to the service, where 54 of them pass byte-identical), its depguard rule, and the seeder's provider-platform and learning data. Also dropped: dependencies only the module used (oauth2, bluemonday, gocsv, strutil).Programs' integration tests run against a fake Provider Platforms service.
Token and URL match Open Content: the server and scheduler read
PROVIDER_PLATFORMS_TOKENandPROVIDER_PLATFORMS_URL. The URL defaults tohttp://provider-platforms:8080, and both refuse to start without the token, outside tests.Compose and make, like Open Content: an
x-provider-platformsanchor builds the service from GitHub (PROVIDER_PLATFORMS_REF, defaultmain), or from your checkout withPROVIDER_PLATFORMS_DIR. It runs asproviderplatforms-migrate(afterserver-migrate),providerplatformson host port 8083 (Open Content has 8082) andproviderplatforms-worker.make migrate,make migrate-freshandmake seedrun the service'smigrate,migrate-resetandseed-dev, somake seedadds the local Canvas again.Migration renumbered to
00080: main's EN-257 took00079.Related issues: https://unlockedlabs.atlassian.net/browse/EN-206
Merge order (nothing enforces it):
v0.4.0, and pinned here and in unlocked-provider-platforms#1.components.providerPlatforms: trueand the service'ssha-image insites/staging.yaml.pintfor code style and do first run #20 deploys to staging.Staging steps:
pintfor code style and do first run #20 merges,PROVIDER_PLATFORMS_TOKENis in staging'score-secrets.pintfor code style and do first run #20. Staging then runs the service next to the current core, and both schedulers publish the LMS jobs until step 3, so keep the gap short.sha-image on staging, and the oldproviderplatforms-workeris already gone, because chore(style): addpintfor code style and do first run #20 doesn't render it once the component is on.go.mod pins contracts
v0.4.0.